feat: slash commands (built-in, skill, recipe) in acp server#9238
Conversation
* main: (66 commits) Switch GH pages deploy to actions/artifact workflow (#9025) fix(summon): re-apply canonical limits when delegate overrides model (#9183) Split code signing from build (#8587) refactor(logging): consolidate logging setup into shared helper in goose crate (#8817) fix(cli): report cumulative total_tokens in stream-json/json output (#8910) plugins: add open plugins (just skills for now) (#9063) fix(providers): refresh GCP metadata server token on expiration (#8929) chore(deps): bump the cargo-minor-and-patch group across 1 directory with 14 updates (#9178) chore(deps): bump bzip2 from 0.5.2 to 0.6.1 (#8964) chore(deps): bump tauri from 2.10.3 to 2.11.1 in /ui/goose2/src-tauri (#9066) chore(deps): bump hono from 4.12.14 to 4.12.18 in /evals/open-model-gym/mcp-harness (#9073) localize hardcoded strings in provider settings UI (#8931) chore(deps): bump @babel/plugin-transform-modules-systemjs from 7.28.5 to 7.29.4 in /documentation (#9122) move settings into app shell (#9047) Add Location column to CLI skills table (#8785) (feat): add routstr as a declarative provider (#9175) Add FuturMix provider (#8840) fix: convert quoted numeric config values to numbers if needed (#8844) fix(ui): keep SSE reconnect loop alive on long disconnects (#8717) (#8846) fix(openai): apply request_params to outgoing API payload (#9151) ...
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 218e9c8e8c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for c in input.chars() { | ||
| match c { | ||
| '"' if !in_single_quote => in_double_quote = !in_double_quote, | ||
| '\'' if !in_double_quote => in_single_quote = !in_single_quote, |
There was a problem hiding this comment.
Stop treating apostrophes as quote delimiters
split_command_args flips in_single_quote on every ', so ordinary text like O'Reilly is parsed as an unmatched quote and returned as an error. This parser is now used by recipe_slash_command::parse_recipe_args and skills::apply_skill_arguments, so slash-command recipes/skills that accept free-text arguments can fail on common apostrophes even when users are not trying to quote. Please only start single-quote mode when ' begins a quoted token (or otherwise support apostrophes inside unquoted words).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3afe8678fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let known_keys: HashSet<&str> = required | ||
| .iter() | ||
| .chain(optional.iter()) | ||
| .map(|p| p.key.as_str()) |
There was a problem hiding this comment.
Reject required params passed as flags
parse_recipe_args currently treats both required and optional keys as valid --flag names, but the required-position tracking (required_idx) is only advanced by positional tokens. In a mixed call like --component Button old-lib, the positional token is incorrectly assigned back to component, which overwrites the flagged value and leaves from missing. This produces confusing "missing parameter" failures for otherwise reasonable input; either disallow --<required> entirely or mark required params as consumed when seen as flags.
Useful? React with 👍 / 👎.
| for command in super::recipe_slash_command::commands_from_mappings( | ||
| super::recipe_slash_command::list_commands(), | ||
| ) { | ||
| if reserved_names.insert(command.name.clone()) { | ||
| commands.push(command); |
There was a problem hiding this comment.
Reserve names for all mapped recipe commands
The ACP command list only reserves names from recipes that successfully validate in commands_from_mappings, then it exposes same-named skills. Execution still checks raw recipe mappings first and returns a recipe error on invalid files, so users can be shown a skill command that is not actually invokable when an invalid mapped recipe with the same name exists. Reserve names from all configured recipe mappings (or fall back to skill resolution on recipe-validation errors) to keep discovery and execution consistent.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1cfb00b5e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if value.starts_with("--") { | ||
| return Err(anyhow!("Missing value for --{}", flag)); |
There was a problem hiding this comment.
Accept '--' prefixed strings as flagged recipe values
The flag parser currently rejects any flag value whose token starts with --, so valid quoted inputs like /deploy service --args "--dry-run --force" are treated as Missing value for --args even though a value was provided. This blocks a common recipe pattern where a parameter is meant to carry downstream CLI options, because split_command_args strips quotes before this check and the value still begins with --.
Useful? React with 👍 / 👎.
| fn builtin_input_hint(command: &str) -> Option<&'static str> { | ||
| match command { | ||
| "prompt" => Some("<name> [--info] [key=value...]"), | ||
| "prompts" => Some("[--extension <name>]"), |
There was a problem hiding this comment.
Align
/prompts input hint with actual argument parsing
The ACP command metadata advertises /prompts [--extension <name>], but command execution does not parse a --extension flag (it treats the first token as the raw extension name in handle_prompts_command). Following the published hint therefore fails with an "Extension '--extension' not found" style error instead of filtering prompts, so discovery metadata and runtime behavior diverge.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f30d691a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self.send_available_commands_update(cx, &args.session_id) | ||
| .await?; | ||
|
|
||
| Self::send_available_commands_update(cx, &args.session_id, &args.cwd)?; |
There was a problem hiding this comment.
Use persisted cwd when publishing load_session commands
on_load_session builds the available-command list from args.cwd, but slash-skill execution later resolves skills from the session’s stored working directory (handle_skill_command reads it via session_manager.get_session). When those differ (for example clients that call load with a generic cwd like "~"), ACP advertises skills that are not actually runnable in that loaded session, causing /skill commands suggested by discovery to fail at runtime. Build the command update from goose_session.working_dir to keep discovery and execution aligned.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I think args.cwd is intentional here. on_load_session updates the persisted session working_dir before publishing commands:
self.session_manager
.update(&session_id)
.working_dir(args.cwd.clone())
.apply()
.await?;
Slash-skill execution later reloads the session from session_manager, so it should resolve skills from that same updated cwd.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6dbcad75a7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for c in input.chars() { | ||
| match c { | ||
| '"' if !in_single_quote => in_double_quote = !in_double_quote, | ||
| '\'' if !in_double_quote && (in_single_quote || current.is_empty()) => { |
There was a problem hiding this comment.
Honor single-quoted substrings inside tokens
The new single-quote guard only enters quote mode when current.is_empty(), so inputs like ENV='two words' cmd or --note='needs review' are tokenized at the internal space instead of being kept as one argument. This is a regression from the previous parser behavior and now affects every caller of split_command_args (notably stdio extension parsing plus recipe/skill slash-argument parsing), causing valid quoted values attached to env/flag prefixes to be misparsed or rejected.
Useful? React with 👍 / 👎.
jamadeo
left a comment
There was a problem hiding this comment.
Really nice. Only comments are things I noticed are pre-existing.
For recipe slash commands, I also wonder why we put them in the user config instead of the recipe itself as a slash-command field
| } | ||
| } | ||
|
|
||
| output.push_str("\n---\nThis knowledge is now available in your context."); |
There was a problem hiding this comment.
I know this is pre-existing but I don't really see why we need to say to the model "This is now in your context"
There was a problem hiding this comment.
I was wondering about this too! I'll remove it and check there are any difference
| if crate::slash_commands::get_recipe_for_command(cmd).is_some() { | ||
| if crate::slash_commands::recipe_slash_command::get_recipe_for_command(cmd) | ||
| .is_some() | ||
| { |
There was a problem hiding this comment.
also pre-existing, but feels like this tracking should be inside Agent.execute_command after we parse
| if crate::slash_commands::get_recipe_for_command(cmd).is_some() { | ||
| if crate::slash_commands::recipe_slash_command::get_recipe_for_command(cmd) | ||
| .is_some() | ||
| { |
| load it first to get the detailed instructions.\n\n\ | ||
| Examples:\n\ | ||
| - load_skill(name: \"gdrive\") → Loads the gdrive skill instructions\n\ | ||
| - load_skill(name: \"my-skill\", args: \"the arguments for the skill\") → Loads a skill with arguments\n\ |
There was a problem hiding this comment.
Should we have some structure to the args instead of a string and the REGEX and replace logic in the frontend?
It seems to me like if we had that structure - we could build a nice argument entry experience on the frontend
There was a problem hiding this comment.
Good point on the UX! The reason we matched this shape is portability. Tt mirrors Claude Code and the agentskills.io spec, so skills move between tools without rewriting.
One option: keep the string as the source of truth (required, works everywhere), and let skills optionally declare structured arg metadata that the frontend uses when present. Authors who only care about portability do nothing extra; goose-native skills can opt in for the richer entry experience.
Yes, will think about in the "@agent/recipe" work |
* main: (70 commits) Feat/summon subagent instructions (#9325) feat: open-plugins generalization + skills (#9112) feat(hooks): PreToolUse denial (#9304) Add support for optional api_key configuration for declarative openai-engine providers (#9202) fix(cli): use plain '> ' prompt instead of goose emoji (#9305) flag for login shell PATH (#9313) Remove popular chat topics from new chat screen (#9307) fix: stop killing goosed when a window closes (#9302) Remove vendored Windows binaries (#9318) Add Linux musl CLI builds (#9240) feat(acp): paginate session list (#9199) docs: reorganize (#9310) Structured per-provider config block, non-destructive provider switching (#8977) feat(cli): add `goose review` local code review command (#9114) feat(tui): diff viewer (#9260) fix(otel): emit trace_output as span attribute instead of event (#9255) docs: add guide for connecting goose Desktop to a remote goosed server (#9275) fix(config): check file fallback when keyring has no entry (#9279) fix(desktop): ScheduleModal error message styling (#9278) fix(ui): align sidebar hamburger in macOS fullscreen (#9257) ...
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 685fdd63b1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if required.len() == 1 && !positionals.is_empty() { | ||
| result.push((required[0].key.clone(), positionals.join(" "))); | ||
| } else { | ||
| for (idx, value) in positionals.into_iter().enumerate() { | ||
| if idx >= required.len() { | ||
| return Err(anyhow!("Unexpected positional argument: {}", value)); |
There was a problem hiding this comment.
Preserve single-parameter positional args for optional-only recipes
When a recipe has no required parameters but does have a single optional parameter (for example a common args field), positional slash input now fails with Unexpected positional argument because only required.len() == 1 gets the greedy positional path. In resolve_command, this means /my-recipe some free text is rejected unless rewritten as --args ..., which is a behavior regression from the previous single-parameter flow and can break existing mapped recipe commands.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
logic/rule for recipe params in slash command: required params take positionals, optional params use -- flags. We deliberately removed the args special-case
* main: feat: add /goal command for agent self-evaluation before finishing (#9069)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 447333492a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (['Builtin', 'Recipe', 'Skill'].includes(item.itemType)) { | ||
| return '/' + item.name; |
There was a problem hiding this comment.
Disambiguate skill selection before inserting
/ command
This change makes skill picks insert /<name>, but the slash-command feed used by this popover can return duplicate names across Builtin/Recipe/Skill. In that case, selecting a Skill entry can execute a different command type because runtime resolution checks built-ins and recipes before skills, so the UI selection is no longer reliable for colliding names (e.g. a skill named prompt). Please dedupe/disambiguate command names (or preserve non-slash insertion when ambiguous) before emitting /<name>.
Useful? React with 👍 / 👎.
| let name = normalize_command_name(&source.name); | ||
| if name.is_empty() { |
There was a problem hiding this comment.
Skip non-token skill names in slash command export
commands_from_sources lowercases skill names but otherwise forwards them as slash command names, which allows entries containing spaces or other token-breaking characters. Slash parsing only takes the first token as the command, so a discovered skill like Code Review is advertised but cannot be invoked as /code review (it parses as command code). Filter/normalize to command-safe names before publishing these entries.
Useful? React with 👍 / 👎.
* main: (38 commits) [Prompt injection mitigation] Update pattern-based detection to reduce FPs (#9350) feat: add Harbor eval runner (#9138) chore(release): bump version to 1.35.0 (minor) (#9150) Include request URL in provider error messages (#9232) fix(databricks): ensure parallel tool image responses don't interleave tool results (#9241) Surface resolved Databricks model metadata (#9206) Add unified thinking effort control across all providers (#9242) Add Linux desktop Vulkan packages (#9323) chore: update canonical model registry (#9331) feat: slash commands (built-in, skill, recipe) in acp server (#9238) feat: add /goal command for agent self-evaluation before finishing (#9069) Feat/summon subagent instructions (#9325) feat: open-plugins generalization + skills (#9112) feat(hooks): PreToolUse denial (#9304) Add support for optional api_key configuration for declarative openai-engine providers (#9202) fix(cli): use plain '> ' prompt instead of goose emoji (#9305) flag for login shell PATH (#9313) Remove popular chat topics from new chat screen (#9307) fix: stop killing goosed when a window closes (#9302) Remove vendored Windows binaries (#9318) ...
Summary
Wires
builtin,skill, andrecipeslash commands into the ACPavailable_commandsupdate so ACP clients get a unified list, and restructures the slash-command code into a proper module with first-class skill and recipe handlers (incl. argument parsing).Changes
ACP: surface all slash commands
available_commands_updatenow returns builtins + recipes + skills in one list, with name-collision precedencebuiltin > recipe > skill.MentionPopovernow inserts/skill-namefor skill items, matching builtin/recipe behavior (previously inserted prose"Use the X skill to ").New
slash_commands/moduleReplaces the old single-file
crates/goose/src/slash_commands.rswith a module that splits recipe, skill, into their own files and merges them into a unified list for ACP.Skill slash commands
/skill-name [args].$ARGUMENTSblob, positional tokens ($1,$ARGUMENTS[0]), or named args ($component,$from) tied to the skill's declared argument names. Skills with no placeholders still receive the raw args appended so the model can see them.metadata.argument-hintandmetadata.arguments(per the agentskills.io frontmatter spec). Open question whether to promote these to top-level frontmatter to match Claude Code — happy to discuss.argument-hintfrom frontmatter is surfaced as the ACPinput_hint.load_skillMCP tool and the slash-command path. Theload_skilltool also accepts an optionalargsfield.Recipe slash commands
--flag value. Example:/migrate "Button Group" old-lib --to new-lib./weather Melbourne weatherworks without quoting).<required1> <required2> [--optional <optional>]and shown in the ACP command list.build_recipe_from_template_with_positional_paramshelper.slash_commands. Open question whether to move these to a per-project / discoverable location like skills — happy to discuss.Misc cleanup
split_quotedfromgoose-clito sharedgoose::utils::split_command_args; reused by stdio extension parsing, recipe args, and skill args. Now also tolerates apostrophes inside unquoted words (O'Reilly,don't).handle_skills_command(/skillslisting) now delegates toskill_slash_command::format_installed_skills.Testing
Unit Test
Manual testing in Desktop App
Created a simple client locally to test integrate with acp server
Open questions
argument-hint/argumentslive at the top level of frontmatter (Claude Code style) instead of undermetadata:?Next